Implement some string functions for libzigc #23847

RcCreeperTech · 2025-05-10T04:14:04Z

Progress towards #2879.

Add memchr, memrchr, strchr, strchrnul, strrchr, index, rindex, memset, memccpy, memmem, mempcpy, stpcpy, stpncpy, strcat, strcpy, strncpy, strnlen
Remove memcmp and depend on compiler_rt implementation

lib/c/string.zig

lib/compiler_rt/memset.zig

lib/c/string.zig

rpkak

I thought about creating a PR with functions like this a few weeks ago but decided not to start working on it until #23538 is resolved because I don't want to make a nearly unnoticeable diversion between libzigc and libc. Since this PR exists now, I tested it against "libc-test/src/functional/string*.c" and found the following error:

src/functional/string_strchr.c:73: strchr(s,128) with align=0 returned 0, wanted str+127
src/functional/string_strchr.c:73: strchr(s,128) with align=1 returned 0, wanted str+127
src/functional/string_strchr.c:73: strchr(s,128) with align=2 returned 0, wanted str+127
src/functional/string_strchr.c:73: strchr(s,128) with align=3 returned 0, wanted str+127
src/functional/string_strchr.c:73: strchr(s,128) with align=4 returned 0, wanted str+127
src/functional/string_strchr.c:73: strchr(s,128) with align=5 returned 0, wanted str+127
src/functional/string_strchr.c:73: strchr(s,128) with align=6 returned 0, wanted str+127
src/functional/string_strchr.c:73: strchr(s,128) with align=7 returned 0, wanted str+127
src/functional/string_strchr.c:74: strchr(s,255) with align=0 returned 0, wanted str+254
src/functional/string_strchr.c:74: strchr(s,255) with align=1 returned 0, wanted str+254
src/functional/string_strchr.c:74: strchr(s,255) with align=2 returned 0, wanted str+254
src/functional/string_strchr.c:74: strchr(s,255) with align=3 returned 0, wanted str+254
src/functional/string_strchr.c:74: strchr(s,255) with align=4 returned 0, wanted str+254
src/functional/string_strchr.c:74: strchr(s,255) with align=5 returned 0, wanted str+254
src/functional/string_strchr.c:74: strchr(s,255) with align=6 returned 0, wanted str+254
src/functional/string_strchr.c:74: strchr(s,255) with align=7 returned 0, wanted str+254

It is caused by @as(c_char, @bitCast(@as(u8, 128))) == -128 != 128.

lib/c/string.zig

alexrp · 2025-05-12T13:52:32Z

Has conflicts after #23835 due to files being moved; a rebase should be enough to deal with it automatically.

lib/c/string.zig

alexrp · 2025-05-12T14:45:39Z

In hindsight, the way I set things up in lib/c.zig didn't really make sense:

zig/lib/c.zig

Lines 16 to 34 in 833d4c9

    
           comptime { 
        
               if (builtin.target.isMuslLibC() or builtin.target.isWasiLibC()) { 
        
                   // Files specific to musl and wasi-libc. 
        
                   _ = @import("c/string.zig"); 
        
                   _ = @import("c/strings.zig"); 
        
               } 
        
               if (builtin.target.isMuslLibC()) { 
        
                   // Files specific to musl. 
        
               } 
        
               if (builtin.target.isWasiLibC()) { 
        
                   // Files specific to wasi-libc. 
        
               } 
        
               if (builtin.target.isMinGW()) { 
        
                   // Files specific to MinGW-w64. 
        
               } 
        
           }

I suggest you move those two @imports above any of the libc-specific blocks, and instead cover the @exports with appropriate builtin.target.is*LibC() checks in lib/c/string[s].zig. In particular, most of the exports aren't needed for MinGW - only mempcpy and strnlen at this time, AFAICT.

lib/c/string.zig

- Add memchr, memrchr, strchr, strchrnul, strrchr - Remove memcmp and depend on compiler_rt implementation

Add index, rindex, memset

strrchr will return the address of the null terminator it c == 0

- add memccpy, memmem, mempcpy, stpcpy, stpncpy, strcat, strcpy, strncpy, strnlen

- Correct pointer types for alignment - Use stdlib functions for `memchr` and `memrchr` - Remove `mempcpy` and `strnlen` implementaitons from `lib/libc/mingw` - Remove duplicate `__aeabi_mem*` symbols

- fix tests for `strchr` and `strrchr`

lib/c/strings.zig

lib/c/string.zig

alexrp · 2025-05-15T09:01:06Z

This'll need a rebase due to #23840 being merged, but feel free to wait until I actually review the function implementations here.

alexrp · 2025-05-15T09:03:23Z

lib/c/string.zig

+fn stpcpy(noalias dest: [*:0]c_char, noalias src: [*:0]const c_char) callconv(.c) [*:0]c_char {
+    var d: [*:0]c_char = dest;
+    var s: [*:0]const c_char = src;
+    // QUESTION: is std.mem.span -> @memset more efficient here?


I think you'd have to just measure this with a few different array sizes.

alexrp · 2025-06-01T18:43:44Z

lib/c/string.zig

+    try std.testing.expect(strrchr(foo, 0) == (foo + 5));
+}
+
+fn __aeabi_memclr(dest: *anyopaque, n: usize) callconv(.c) *anyopaque {


This looks unused? We also have this in compiler-rt anyway.

andrewrk · 2025-06-02T17:45:26Z

lib/c/string.zig

+}
+
+fn memccpy(noalias dest: *anyopaque, noalias src: *const anyopaque, c: c_int, n: usize) callconv(.c) ?*anyopaque {
+    @setRuntimeSafety(false);


why would this need to be different than the mode set for the compilation unit?

andrewrk · 2025-06-02T17:46:37Z

lib/c/string.zig

+    const d: [*]c_char = @ptrCast(dest);
+    const s: [*]const c_char = @ptrCast(src);


no need to pointer cast; you can change the parameter types.

My rationale for this was that I wanted to keep the function signature as close to the c signature as possible. I was basing this decision off of a comment that you made in a prior post here. Technically in this case this would actually increase the type information, but it would not match 1:1 with the c ABI so I am unsure which approach is better.

Thank you, I forgot about that. If you don't mind, a comment would help remind future me, and future contributors as well, of the rationale behind this.

andrewrk · 2025-06-02T17:47:36Z

lib/c/string.zig

+    @setRuntimeSafety(false);
+    const d: [*]c_char = @ptrCast(dest);
+    const s: [*]const c_char = @ptrCast(src);
+    const needle: c_char = @intCast(c);


please add a comment explaining why the @intCast is justified (does the spec say what happens if you pass an int that does not fit into a char?)

andrewrk · 2025-06-02T17:48:30Z

lib/c/string.zig

+    var idx: usize = 0;
+    while (idx < n) : (idx += 1) {
+        d[idx] = s[idx];
+        if (d[idx] == needle) return d + idx + 1;
+    }


this looks like it could be implemented better using already existing std.mem functions, which are SIMD optimized.

andrewrk · 2025-06-02T17:49:09Z

lib/c/string.zig

+    const src: []const u8 = "supercalifragilisticexpialidocious";
+    var dst: [src.len]u8 = @splat(0);
+    const dOffset = std.mem.indexOfScalar(u8, src, 'd').?;
+    const endPtr: *anyopaque = @ptrCast(@as([*]u8, &dst) + dOffset + 1);


no need to use @ptrCast in a unit test.

andrewrk · 2025-06-02T17:50:38Z

Since this is your first time contributing, I suggest to start with only one function. After you get the hang of it, it could make sense to make a larger PR.

andrewrk requested changes May 10, 2025

View reviewed changes

lib/c/string.zig Outdated Show resolved Hide resolved

RcCreeperTech commented May 10, 2025

View reviewed changes

lib/c/string.zig Outdated Show resolved Hide resolved

alexrp reviewed May 10, 2025

View reviewed changes

lib/compiler_rt/memset.zig Outdated Show resolved Hide resolved

alexrp reviewed May 10, 2025

View reviewed changes

lib/c/string.zig Outdated Show resolved Hide resolved

alexrp reviewed May 10, 2025

View reviewed changes

lib/c/string.zig Outdated Show resolved Hide resolved

alexrp reviewed May 10, 2025

View reviewed changes

lib/c/string.zig Outdated Show resolved Hide resolved

alexrp reviewed May 10, 2025

View reviewed changes

lib/c/string.zig Outdated Show resolved Hide resolved

rpkak suggested changes May 10, 2025

View reviewed changes

lib/c/string.zig Outdated Show resolved Hide resolved

SeanTheGleaming reviewed May 10, 2025

View reviewed changes

lib/c/string.zig Outdated Show resolved Hide resolved

alexrp reviewed May 12, 2025

View reviewed changes

lib/c/string.zig Outdated Show resolved Hide resolved

alexrp reviewed May 12, 2025

View reviewed changes

lib/c/string.zig Outdated Show resolved Hide resolved

RcCreeperTech added 9 commits May 12, 2025 16:05

Implement some string functions for libzigc

39e66ee

- Add memchr, memrchr, strchr, strchrnul, strrchr - Remove memcmp and depend on compiler_rt implementation

Implement more string functions

328b64d

Add index, rindex, memset

Fix test for strrchr.

37e67d3

strrchr will return the address of the null terminator it c == 0

Fix: Correct strchr implementation

c84c5ff

fix: remove wrappers index, rindex

df80d50

Implement Several More Functions

f90a588

- add memccpy, memmem, mempcpy, stpcpy, stpncpy, strcat, strcpy, strncpy, strnlen

Fix: conflit with memset in compiler_rt

8ef0b92

Fix: type alignment and mingw

1afe953

- Correct pointer types for alignment - Use stdlib functions for `memchr` and `memrchr` - Remove `mempcpy` and `strnlen` implementaitons from `lib/libc/mingw` - Remove duplicate `__aeabi_mem*` symbols

fix: re-organize imports

Loading
Loading status checks…

88452ae

- fix tests for `strchr` and `strrchr`

RcCreeperTech force-pushed the libzigc branch from 492df3e to 88452ae Compare May 13, 2025 07:37

alexrp reviewed May 13, 2025

View reviewed changes

lib/c/strings.zig Show resolved Hide resolved

tauoverpi reviewed May 14, 2025

View reviewed changes

lib/c/string.zig Outdated Show resolved Hide resolved

Fix: hidden symbol exports, and remove redundant @ptrCasts

Loading
Loading status checks…

e5e2a58

alexrp reviewed May 15, 2025

View reviewed changes

alexrp mentioned this pull request May 17, 2025

libc: implement common abs for various integer sizes #23893

Merged

alexrp self-assigned this May 17, 2025

alexrp reviewed Jun 1, 2025

View reviewed changes

andrewrk requested changes Jun 2, 2025

View reviewed changes

		const d: [*]c_char = @ptrCast(dest);
		const s: [*]const c_char = @ptrCast(src);

Implement some string functions for libzigc #23847

Are you sure you want to change the base?

Implement some string functions for libzigc #23847

Conversation

RcCreeperTech commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rpkak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alexrp commented May 12, 2025

Uh oh!

Uh oh!

alexrp commented May 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexrp commented May 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewrk commented Jun 2, 2025

Uh oh!

RcCreeperTech commented May 10, 2025 •

edited

Loading